Skip to content

Comments

Add new django tutorial#4869

Merged
leahecole merged 119 commits intoGoogleCloudPlatform:masterfrom
glasnt:django-run
Dec 2, 2020
Merged

Add new django tutorial#4869
leahecole merged 119 commits intoGoogleCloudPlatform:masterfrom
glasnt:django-run

Conversation

@glasnt
Copy link
Contributor

@glasnt glasnt commented Oct 19, 2020 •
edited
Loading

Description

Implements new Django tutorial with integration testing. (Internal cl/309679445)

Checklist

@glasnt glasnt requested review from a team, averikitsch and grant as code owners October 19, 2020 05:52
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 19, 2020
grant
grant reviewed Oct 19, 2020
View reviewed changes
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Is there any more background to this sample? Will it be included under cloud.google.com/run? If not, maybe we can host the sample in a community tutorial folder with our other cloud run tutorials.

Is the tutorial original? I didn't review the code as it doesn't seem to be originally authored.

CC @grayside

run/django/README.md Outdated
This repository is an example of how to run a [Django](https://www.djangoproject.com/)
app on Google Cloud Run (fully managed).

The Django application used in this tutorial is the [Writing your first Django app](https://docs.djangoproject.com/en/3.0/#first-steps), after completing [Part 1](https://docs.djangoproject.com/en/3.0/intro/tutorial01/) and [Part 2](https://docs.djangoproject.com/en/3.0/intro/tutorial02/)).
Copy link
Contributor

@grant grant Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we 100% sure that Google authored this code and we're able to claim it under this repo's license? It seems like this tutorial should be hosted on Django's website/repo and not here.

Copy link
Contributor Author

@glasnt glasnt Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the pattern from the kubernetes and appengine samples.

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Oct 19, 2020
averikitsch
averikitsch reviewed Oct 19, 2020
View reviewed changes
@glasnt
Copy link
Contributor Author

glasnt commented Nov 23, 2020

Linting was failing since typechecks are currently configured to include typechecking noxfile.py itself, which is copied per workspace from ~/noxfile-template.py.

"⚠️" marked edits attempt to correct this by adding typechecks to noxfile-template.py itself, rather than adding noxfile.py to the ignore list.

Happy to cherrypick these comments to a different branch and rebase this branch, for code cleanliness. (This branch also needs a squash, maybe, not seeing anything in AUTHORING_GUIDE about such things.

averikitsch
averikitsch approved these changes Nov 23, 2020
View reviewed changes
Copy link
Contributor

@averikitsch averikitsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm pending test passing

@averikitsch averikitsch removed the request for review from engelke November 24, 2020 22:26
@glasnt glasnt requested a review from leahecole November 24, 2020 23:24
leahecole
leahecole requested changes Dec 2, 2020
View reviewed changes
Copy link
Collaborator

@leahecole leahecole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor changes! Ping me when you're ready for what is most likely the final review

@leahecole leahecole merged commit ee0d786 into GoogleCloudPlatform:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@averikitsch averikitsch averikitsch approved these changes

@leahecole leahecole leahecole approved these changes

+2 more reviewers

@grant grant grant left review comments

@kurtisvg kurtisvg kurtisvg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@leahecole leahecole

Labels

cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@glasnt @averikitsch @grant @leahecole @kurtisvg @kokoro-team